Skip to content

Porting TokenizingTextBox#77

Merged
Arlodotexe merged 30 commits intomainfrom
niels9001/TokenizingTextBox
Jul 11, 2023
Merged

Porting TokenizingTextBox#77
Arlodotexe merged 30 commits intomainfrom
niels9001/TokenizingTextBox

Conversation

@niels9001
Copy link
Collaborator

@niels9001 niels9001 commented Jun 3, 2023 •
edited
Loading

This PR ports the TokenizingTextBox to the new repo, addressing: #53

Changes:

  • Ported over code, sample and tests.
  • Visual update to the TokenizingTextBox to make it inline with WinUI styling:

Before:
image

After:
image

  • Working on WASDK and UWP - couldn't figure out why it's not showing the tokens on WASM. Might be the ItemsPresenter issue we were having with SettingsExpander as well?

@niels9001 niels9001 requested a review from Arlodotexe June 7, 2023 12:24
@niels9001 niels9001 marked this pull request as ready for review June 7, 2023 12:24
@niels9001
Copy link
Collaborator Author

niels9001 commented Jun 7, 2023

@Arlodotexe When copying the WrapPanel code manually into the project, everything works. When referencing the .Primitives.proj I'm getting this error:

image

@niels9001 niels9001 linked an issue Jun 7, 2023 that may be closed by this pull request
Port TokenizingTextBox #53
Closed
@niels9001
Copy link
Collaborator Author

niels9001 commented Jun 21, 2023

@Arlodotexe did you see the crash as mentioned above?

@Arlodotexe
Copy link
Member

Arlodotexe commented Jun 21, 2023

@Arlodotexe did you see the crash as mentioned above?

It looks like you are referencing the primitives .csproj directly via the MSBuild property ToolkitPrimitiveSourceProject, but I'm not seeing the crash you mentioned on any platform.

Uno does appear to have an issue though. We should file a ticket to investigate this issue so it can be fixed from Uno's end.

michael-hawker
michael-hawker reviewed Jul 6, 2023
View reviewed changes
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't try testing locally yet, but just going through a few things to get up to speed. Did a comparison with Uno fork and didn't notice any big differences. I know originally, I had tested this control on Android for the Graph components, but don't think I had tried WASM at the time.

@niels9001 @michael-hawker
Update components/TokenizingTextBox/src/InterspersedObservableCollect…
34e8ab6 
…ion.cs

Co-authored-by: Michael Hawker MSFT (XAML Llama) <24302614+michael-hawker@users.noreply.github.com>
@Arlodotexe Arlodotexe mentioned this pull request Jul 10, 2023
Closed
@Arlodotexe Arlodotexe merged commit 56dff69 into main Jul 11, 2023
@delete-merged-branch delete-merged-branch bot deleted the niels9001/TokenizingTextBox branch July 11, 2023 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Arlodotexe Arlodotexe Arlodotexe approved these changes

@michael-hawker michael-hawker michael-hawker left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Port TokenizingTextBox

3 participants

@niels9001 @Arlodotexe @michael-hawker